Ensure FilterSomaticVcf handles PASS variants correctly #909
Conversation
|
|
||
| val allFilters = { | ||
| if (variant.filters == PassingFilters && newFilters.nonEmpty) newFilters.toSet | ||
| else variant.filters ++ newFilters |
There was a problem hiding this comment.
Appending filters to a PASS in a variant seems like an easy mistake (which I've made!)
What do you think of a helper that would perform a variant copy and handle PASS correctly like:
Variant.withFilters()Variant.withFilter()
The plural would add all filters in a single copy. The singular would just add one filter.
There was a problem hiding this comment.
I think I like the plural one, but not the singular. Since it involves calling copy() I worry a little that we'd end up seeing code like v.withFilter(x).withFilter(y).withFilter(z) etc.
Codecov ReportPatch coverage:
📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more Additional details and impacted files@@ Coverage Diff @@
## main #909 +/- ##
=======================================
Coverage 95.65% 95.65%
=======================================
Files 126 126
Lines 7294 7296 +2
Branches 487 480 -7
=======================================
+ Hits 6977 6979 +2
Misses 317 317
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
|
|
||
| val allFilters = { | ||
| if (variant.filters == PassingFilters && newFilters.nonEmpty) newFilters.toSet | ||
| else variant.filters ++ newFilters |
There was a problem hiding this comment.
I think I like the plural one, but not the singular. Since it involves calling copy() I worry a little that we'd end up seeing code like v.withFilter(x).withFilter(y).withFilter(z) etc.
|
Thanks @tfenne! I decided not to implement the helper method since it would do 1 copy, then I would need to do another copy to override the new |
If the tool was run on data that had
PASSin the FILTER field then the output could be:ATailingArtifact;EndRepairFillInArtifact;PASSWhen the expected output is:
ATailingArtifact;EndRepairFillInArtifactThis PR handles variants with
PASScorrectly by unsettingPASSif new filters are added.